This is one I'll need to pull down and play with to put it through it's paces, but moving that to the host here should work for all considerations I can think of.

It worked for the ones I considered as well. There's a note in the spec about why it was structured that way, but I couldn't find a way it didn't work like this.

These are both typically in the same spot because an item is either a menuitem, menuitemcheckbox, or menuitemradio, so I'm wondering if we can simplify this with something that serves a single indicator but sets the class and part based on its role. So a single conditional check with the part/class set based on the role. Thoughts? I think it simplifies the template and still accomplishes the same thing.

Actually, I think we may only need one of these and I think it should likely still be called checked-indicator.

While I know that the initial fast-components made these look like a checkbox and an radio, the actual indicator represents the checked state of either an element with one of the checkable roles. I'd like to dig in here, because I think this can simplify and while someone may choose to make it look like one or the other, that's also primarily state based that can be handled in styles or using a when directive from the view to manage sending alternative svg's here. In other implementations, it may be the same SVG, but the behavior only allows single selection.

See my comment on progress for one potential way to slim down this block by removing the slot. I don't see a case for individual components to slot in a custom indicator here.

I do think it's beneficial to be able to provide the icons individually, the same icon if that's what your design called for, but I think it's good UX to have different icons. I'd hate to see the css get messier to accommodate a minor reduction.

Again, I don't think that is prohibited by having a singular slot. These could be handled in configuration using a when directive without needing to hoist that requirement to all implementations. If the goal is whether we can accomplish what you've stated above I think we should use the most concise method of addressing that. I commented below but the removal of slots in general is a non-starter. The options don't replace it, they provide a method of configuring a global default. That helps for "menu" but if I need to configure something else there in a one-off scenario, the slot is still massively more useful.

A couple of thoughts here.

• Let's decouple the icons as mentioned above.
• Should we make raio/checkbox support an option and only compose those in if present? We could make this really simple. See the label options sampele above.

If we keep radio/checkbox in by default, I think we should remove the use of the double when and instead just render a property that returns a template based on a single condition check. That will be slightly better in terms of performance.

Yes ^ - My big question here is whether the better ergonomic is baking the conditional rendering in or letting that be handled by the person implementing. On one hand, in the case I don't need two I need to pass the same thing twice. Alternatively, if I have two different SVG's I can handle my own when, right? Does one match our composition value more than the other?

I'd love to find a way to slim down the template here if that's what @EisenbergEffect's last comment suggests. I'm not sure I am seeing the exact change though.

From a component framework perspective, the person authoring the component doesn't know if someone is going to use checkbox or radio or not. Since the component supports this to fully support the ARIA spec it should also have a clear way to have two different indicators. This should be easy without having to worry about it for each menu you implement.

I've consolidated this to a single conditional using the pattern from Flipper.

I understand that someone may not always need checked functionality, and the only feasible solution I see for that is using the template builder to exclude it. For now we have that role capability built in.

From a visual perspective, it's bad usability for two things that operate differently to have the same look, so I can't in good conscience agree with a solution where the easy default is to treat them the same. The fact that they both relate to the checked state is not the sole decision or the ARIA spec could have omitted the two roles and encouraged people to implement radio functionality in code.

This is at least better than what we had, and if we want to consider changing this, though my vote is not to, we should pull it into a new issue.

@EisenbergEffect do you think this and the below code snippet might be a good candidate for one of the new conditional directives?

Note that the expanded check changes with the menu updates i have in the RFC for removing anchored region...

It's actually simpler than that I think. We don't need render to do this. Instead, we can have a single computed property that based on the conditions returns just one template. That will be cheaper than multiple when instances.

Larger question here is whether it's worth keeping the attribute for horizontal and vertical. If we decide to keep it there is an argument for keeping a class here out of convenience. If we opt for removal then the primary method of adding would require extending the class (which I don't think is the worst thing). Given that there isn't a functional purpose, I'd be for removing the attribute from here as vertical and horizontal is primarily a visual consideration which would track similar to appearances on other controls.

If we remove this, should we have the CLI set up the default by adding this back in? I saw a customer just the other day indicating that this was important. So, if we remove it from the base, I'm thinking it might be nice to add it via the CLI.

Yes - And on this, the confusion I believe is that the attribute exists on the Foundation component but then no styling is delivered. I'd propose that vertical in this case is largely a design consideration. There's no functional requirement for it and no impact to AT currently by it existing. One could add an attribute for it by extending the component (what we should do in the CLI), perhaps they want to use data- instead to just apply the visual style. I think the attribute can be removed from Foundation but it's a good example on how to extend and a pattern we value, so a good fit for the CLI implementation. Open here, but I do think that it's confusing to have this in Foundation when it only serves a visual purpose. That follows along with not baking semantics on appearances into foundation.

The aria-orientation attribute is an inherited property for the radiogroup role. A note for the attribute indicates that ARIA 1.1 changed the default value from being horizontal to undefined: https://w3c.github.io/aria/#radiogroup:~:text=In%20ARIA%201.1%2C%20the%20default,ambiguous%20(e.g.%2C%20radiogroup).

It can still be passed to the host where the role is, we just don't need to actually use it within the class or template, so I'm not sure it needs to be enumerated.

The goal in removing it was bringing the selectors closer together. There have been some specificity issues, though this may not be one of them. Since there was no functional changes based on the orientation, using the attribute in the selector seemed like a good way to simplify the overall component. I suppose that could be said for all class selectors, though we've decided they are worth the overhead for internal styling convenience. By that logic, perhaps it's best to leave this here.

Revisiting this and I don't think we want these within the button itself because it will group all content as part of the label. While I can definitely see a case for the expand-collapse-icon, moving the others in could actually generate some new a11y issues for folks under the assumption that the only slotted content here is icons (which I don't think would be an accurate assumption).

This might be a case of an ambiguous definition in the ARIA spec. At https://w3c.github.io/aria-practices/#accordion, under "Roles, States, and Properties" bullet 2.2 says "The button element is the only element inside the heading element." Agreed that if slotted content is something else like a Menu Button with interactions for the section it might confuse what is announced as the title. I believe this is true for the start and end slot placement in other components as well, though maybe less likely that we expect content other than icons. Do we have any general guidance re: a11y on slotted content?

I tested this on Anchor, for instance with a Badge, and it reads the badge content the same in both cases. In some cases that may make sense, like announcing something as "New". If they are Buttons instead, focus goes to the accordion button, then tab to start, end, then the next accordion item, which seems like the flow I expect.

Note that this change (at least currently) is only the sequence of items, not the hierarchy. This should not have any a11y impact that I know of, it only seeks to align the named intent of the "start" and "end" slots with their document order to minimize the need to resequence with css.

I think the intent with not including these as part of the button was that there may be scenarios where people may place other interactive elements within these slots and placing those inside the button would create a problem there. I think this is a hierarchy change because these are moving to be nested within the button, which means anything projected into the slot is a child of the button now instead.

Uh, looks like I didn't look at the diff correctly. Of course those elements moved.

I looked back at the spec, and I don't see a way to make both of the following statements true given the additional elements we have. I agree that the previous hierarchy (I've shifted the sequence) is best.

The button element is the only element inside the heading element.
That is, if there are other visually persistent elements, they are not included inside the heading element.

this is a nit but I'd suggest a span here as the element is intended to be inline.

@bheston I need to pull down and test a couple things here on the a11y end of things. I think it's fine but need to validate the tree with certain slotted items.